-
Notifications
You must be signed in to change notification settings - Fork 558
Specify temporary lifetime extension through expressions #2051
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| r[destructors.scope.lifetime-extension.exprs.extending] | ||
| For a let statement with an initializer, an *extending expression* is an | ||
| expression which is one of the following: | ||
| An *extending expression* is an expression which is one of the following: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A major change here: an extending expression is now any expression that preserves lifetime extension, defined non-inductively. I found this helps with generalizing the definition beyond let statement initializers, but I also often found myself having to refer to an expression being "extending when its parent is extending"; that's are now just an extending expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this greatly widens the definition of an extending expression, putting more load on the other rules.
| * The operand of an extending [borrow] expression. | ||
| * The [super operands] of an extending [super macro call] expression. | ||
| * The operand(s) of an extending [array][array expression], [cast][cast | ||
| * The initializer expression of a `let` statement or the body expression of a [static][static item] or [constant item]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something slightly weird here: the last expression of a const block morally should be here, but it'd be a bit messy to have to exclude it from the rule for blocks lower down. Given that this definition of extending expressions doesn't care about where scopes are extended to, it shouldn't be a semantic issue, but it might warrant reformatting and/or an admonition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see the admonition for this (with one or more examples).
| r[destructors.scope.lifetime-extension.exprs.parent] | ||
| If a temporary scope is extended through the scope of an extending expression, it is extended through that scope's [parent][destructors.scope.nesting]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I struggled a bit with how to express this; in its current form it's a bit of a hack. Ideally, I feel like it wouldn't need to be a separate rule or to refer to the definition of scope nesting, but it's working around something subtle: by ensuring that expressions' temporary scopes are only extended by their scope-ancestors, we can work around const blocks having parent expressions that (to my understanding) shouldn't be considered ancestor scopes of the const block's body; temporaries extended by const blocks are extended to the end of the program1. Maybe there's a simpler way to express this, and regardless it could probably use an admonition.
I do think that some sort of "extended by" or "extended through" or "extending based on" relation is necessary though, regardless of how exactly we choose to define/present it. I feel there's too much ambiguity if we can't precisely associate expressions we're extending the temporary scopes of with the scopes they're being extended to.
Footnotes
-
This PR doesn't make all the changes needed to iron that out, but see Further specify temporary scoping for
statics andconsts #2041. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see the admonition that you have in mind here (with one or more examples). Let's also add rules to define what it means exactly to "extend to", "extend through", etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In particular, what it means to "extend through" a parent scope is going to be worth careful explanation and one or more examples in an admonition.
| ```rust,edition2024 | ||
| # fn temp() {} | ||
| # fn use_temp(_: &()) {} | ||
| // The final expression of a block is extending. Since the block below | ||
| // is not itself extending, the temporary is extended to the block | ||
| // expression's temporary scope, ending at the semicolon. | ||
| use_temp({ &temp() }); | ||
| // As above, the final expressions of `if`/`else` blocks are | ||
| // extending, which extends the temporaries to the `if` expression's | ||
| // temporary scope. | ||
| use_temp(if true { &temp() } else { &temp() }); | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some additional examples would probably be good. Maybe it would help to have one where temporaries are extended through a block but not to the end of a statement? That could also be used as a compile_fail example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it could also use some additional text (or even an admonition?) to make clear the interaction with if block scopes and Rust 2024's tail expression scopes. I'm not sure exactly how much explaining is needed for that, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all rather subtle. Erring on the side of more examples and explanation will be better here.
| r[destructors.scope.lifetime-extension.exprs.let] | ||
| A temporary scope extended through a `let` statement scope is [extended] to the scope of the block containing the `let` statement ([destructors.scope.lifetime-extension.let]). | ||
|
|
||
| r[destructors.scope.lifetime-extension.exprs.static] | ||
| A temporary scope extended through a [static][static item] or [constant item] scope or a [const block][const block expression] scope is [extended] to the end of the program ([destructors.scope.lifetime-extension.static]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if there's a way to cut down on the duplication here. I felt these rules were necessary to be precise about where temporaries' scopes are extended to, but having them in the introduction to the lifetime extension feels necessary too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed they felt necessary here.
| r[destructors.scope.lifetime-extension.exprs.other] | ||
| A temporary scope extended through the scope of a non-extending expression is [extended] to that expression's [temporary scope]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternative to de-duplicating the above two rules, maybe there should be a more detailed section alongside destructors.scope.lifetime-extension.let and destructors.scope.lifetime-extension.static for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean here exactly.
|
Thanks @dianne; will have a look. |
|
@ehuss and I looked carefully through this today. Some thoughts. After staring at this awhile, I see why this makes it easier to express the core idea of rust-lang/rust#146098 -- that the relative drop order within an expression should be consistent regardless of where that expression is. Expressing the rules inductively for defining an extending expression ends up pushing against this. So it does, I think, make a deep sort of sense to drop the inductive approach, as this PR does. At the same time, the inductive approach was carrying a lot of load. The reader could visualize walking down an expression, outside to inside, at each step checking whether it was still an extending expression. Then there was essentially one key rule that leaned on that:
What this PR does, in dropping the inductive framing, is to widen the definition of an extending expression significantly, and thereby put a lot more load on the rules that follow to narrow this back down. That's where it ends up getting a bit subtle. With this approach, both @ehuss and I found building intuition and checking the correctness of the rules to be more difficult. So, in dropping the inductive approach, I think we have some work to do in order to recover the same level of clarity. Adding some I'll leave some other thoughts inline. |
| A temporary scope extended through a [static][static item] or [constant item] scope or a [const block][const block expression] scope is [extended] to the end of the program ([destructors.scope.lifetime-extension.static]). | ||
|
|
||
| r[destructors.scope.lifetime-extension.exprs.other] | ||
| A temporary scope extended through the scope of a non-extending expression is [extended] to that expression's [temporary scope]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rule is particular is one where I wonder how we might do better. The idea, currently, is that the rules above define when a temporary scope would be "extended through" the scope of an expression, and then this rule says, well, if it's a non-extending expression, then we only "extend to" it.
| The operand of an extending [borrow] expression has its [temporary scope] [extended]. | ||
| The [temporary scope] of the operand of a [borrow] expression is *extended through* the scope of the borrow expression. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit subtle that, after having defined above what is an extending expression, that this rule then doesn't use that definition at all. It makes sense -- all operands of any borrow expression are extending. But it'll be worth adding an admonition under each of these rules that elaborates the rationale and gives one or more examples.
|
I'll try and address individual points soon, but first one higher-level note that I'll try to use as guidance when restructuring:
This PR's approach still has a visual intuition to it, so it should be presented in a way that makes that clear: instead of walking down an extending expression to check whether an |
Yes, exactly. This upward walk is what I'd been expecting but it has to be teased out a bit from the presentation here. Hopefully making that more clear will make the rules here more clear. |
Reference PR for rust-lang/rust#146098. This includes a reworked definition of extending expressions with the aim of expressing the new semantics more uniformly.